Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: show pubkey_hex at eotsd keys show and list #320

Merged
merged 6 commits into from
Feb 18, 2025
Merged

Conversation

samricotta
Copy link
Contributor

closes: #274

When using the CLI the user can now retrieve their hex from show and list commands. Added in custom methods to replace the default SDK ones

@samricotta samricotta marked this pull request as draft February 4, 2025 13:00
@samricotta samricotta marked this pull request as ready for review February 4, 2025 15:07
@samricotta samricotta force-pushed the sam/show-pubkey branch 3 times, most recently from dc3728a to 32521de Compare February 12, 2025 12:28
@samricotta samricotta force-pushed the sam/show-pubkey branch 2 times, most recently from 709a0c0 to f902f20 Compare February 14, 2025 17:51
}

listCmd.RunE = runCommandPrintAllKeys
listCmd.Flags().StringP(flags.FlagOutput, "o", flags.OutputFormatText, "Output format (text|json)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion to remove this all good to use --output json

Suggested change
listCmd.Flags().StringP(flags.FlagOutput, "o", flags.OutputFormatText, "Output format (text|json)")

Copy link
Contributor Author

@samricotta samricotta Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm when I remove this though, the test complains when using --output

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it shouldn't since it works locally, maybe there is something weird with the test setup

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    babylon_e2e_test.go:407: 
        	Error Trace:	/home/runner/work/finality-provider/finality-provider/itest/babylon/babylon_e2e_test.go:407
        	Error:      	Received unexpected error:
        	            	flag accessed but not defined: output
        	Test:       	TestPrintEotsCmd

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from the CI

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

humm, I had removed and run the test and it passes TestPrintEotsCmd

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done at #341

Copy link
Contributor

@RafilxTenfen RafilxTenfen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎉

panic("failed to find keys list command")
}

listCmd.RunE = runCommandPrintAllKeys
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could also remove CommandPrintAllKeys since now there is a keys ls subcommand

What do you think @Lazar955 ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep makes sense

@RafilxTenfen RafilxTenfen self-requested a review February 17, 2025 12:08
panic("failed to find keys list command")
}

listCmd.RunE = runCommandPrintAllKeys
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep makes sense

@samricotta samricotta merged commit 2bcf7d5 into main Feb 18, 2025
17 of 18 checks passed
@samricotta samricotta deleted the sam/show-pubkey branch February 18, 2025 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

show pubkey_hex at eotsd keys show and list
3 participants